Skip to content

feat(c): expose TableWrite close for explicit cleanup - #1306

Merged
JingsongLi merged 2 commits into
apache:mainfrom
ZENOTME:feat/c-api-table-write-close
Nov 6, 2026
Merged

JingsongLi merged 2 commits into
apache:mainfrom
ZENOTME:feat/c-api-table-write-close

Conversation

@ZENOTME

@ZENOTME ZENOTME commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Purpose

Expose existing TableWrite::close() through the C API for explicit cleanup. Closes #1302.

Brief change log

Add paimon_table_write_close() using existing C binding conventions. Prepared messages remain caller-owned; free the handle separately.

Tests

Reuse test_write_commit_read_roundtrip: close the writer before committing its prepared messages, then verify the existing readback. No new fixture. Formatting checks pass.

API and Format

One additive C function; no storage format changes.

Documentation

Rustdoc describes ownership and handle lifetime.

@ZENOTME

ZENOTME commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author
thread 'io::cache::disk::shared::tests::batch_download_locks_deduplicate_and_skip_busy_stripes' (14679) panicked at crates/paimon/src/io/cache/shared.rs:1180:9:
assertion `left == right` failed
  left: 0
 right: 2

Looks like unrelatd failure

@QuakeWang

Copy link
Copy Markdown
Member
thread 'io::cache::disk::shared::tests::batch_download_locks_deduplicate_and_skip_busy_stripes' (14679) panicked at crates/paimon/src/io/cache/shared.rs:1180:9:
assertion `left == right` failed
  left: 0
 right: 2

Looks like unrelatd failure

@ZENOTME The unit (ubuntu) failure is the #1218 cache-lock flake, fixed on main in d8b3b11 (#1314). This branch is older than that. Could you rebase on main and push?

@ZENOTME
ZENOTME force-pushed the feat/c-api-table-write-close branch from 103421c to fabf5bd Compare November 4, 2026 12:37
Comment thread bindings/c/src/write.rs Outdated
}
}

/// Close a standard TableWrite, discarding outstanding output without preparing a commit.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Document the I/O and failure semantics of close

#1302 asks for cancellation without flushing unprepared output. This wrapper calls TableWrite::close(), whose append path finishes the active Parquet writer before attempting to delete its file. Cleanup errors are discarded by core and the C function still returns success.

I reproduced this via the C write and close APIs with an instrumented storage provider. The unprepared object was absent before close. Close wrote it and then attempted deletion. With an injected deletion denial, close returned null and the object remained. With successful deletion, it disappeared. In both runs, the previously prepared message still committed and only its row was visible.

Please describe this API as best-effort cleanup that can finish and upload buffered output and that does not report storage cleanup errors. If cancellation without flushing is required for #1302, it needs a separate core abort path.

@ZENOTME
ZENOTME force-pushed the feat/c-api-table-write-close branch 2 times, most recently from 3cf4a06 to 5fa62a0 Compare November 6, 2026 07:47
Comment thread bindings/c/src/write.rs Outdated
}
}

/// Close a standard TableWrite, discarding unprepared output on a best-effort basis.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Document that close may flush buffered output

Please clarify that close() may finish and upload buffered output before attempting deletion. The append path calls writer.close().await during abort, so callers cannot rely on this API for cancellation without flushing. The current text explains cleanup failures but omits this I/O behavior. A documentation clarification is sufficient for this PR.

@ZENOTME
ZENOTME force-pushed the feat/c-api-table-write-close branch from 5fa62a0 to ca31a34 Compare November 6, 2026 08:27

@QuakeWang QuakeWang left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@JingsongLi
JingsongLi merged commit cf56549 into apache:main Nov 6, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Expose TableWrite::close in C bindings

3 participants